Skip to content

Conversation

@shsms
Copy link
Contributor

@shsms shsms commented Mar 5, 2025

This makes it easier to close the streamers when they are no longer
needed.

Also rename the methods from stream_* to *_stream.

shsms added 3 commits March 5, 2025 11:20
They just start a background task.  There's no async until we're
actually waiting for messages.

Signed-off-by: Sahas Subramanian <[email protected]>
This makes it easier to close the streamers when they are no longer
needed.

Also rename the methods from `stream_*` to `*_stream`.

Signed-off-by: Sahas Subramanian <[email protected]>
Now that we are exposing the `GrpcStreamBroadcaster` instances which
can be closed by external parties, we need to check that existing
instances are active before reusing them.

Signed-off-by: Sahas Subramanian <[email protected]>
@shsms shsms requested a review from a team as a code owner March 5, 2025 11:00
@github-actions github-actions bot added the part:tests Affects the unit, integration and performance (benchmarks) tests label Mar 5, 2025
Signed-off-by: Sahas Subramanian <[email protected]>
@github-actions github-actions bot added the part:docs Affects the documentation label Mar 5, 2025
Copy link
Collaborator

@cwasicki cwasicki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

warn_on_overflow: bool = False,
) -> Receiver[Trade]:
) -> GrpcStreamBroadcaster[
electricity_trading_pb2.ReceiveGridpoolTradesStreamResponse, Trade
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that downstream apps need to import these types? Is there a way around this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downstream apps can say: GrpcStreamBroadcaster[Any, PublicTrade] and still be fully typed, because they only care about the output type.

But we can improve during the refactoring.

@shsms shsms added this pull request to the merge queue Mar 5, 2025
Merged via the queue into frequenz-floss:v0.x.x with commit e75c90b Mar 5, 2025
14 checks passed
@shsms shsms deleted the closable-streams branch March 5, 2025 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants